Conversation
…ement a grace period of 15 seconds before rate limiting starts
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive remote debug logging system that captures all console output and submits it to a server endpoint for production debugging. The system includes rate limiting, batch submission, and graceful degradation when the server session expires.
Changes:
- Added remote debug configuration with URL parameter activation (
?debuguser=1&debugkey=<key>) - Implemented console method overrides to capture all log/warn/error calls
- Added rate limiting (20 logs/sec after 10-second grace period) and batch queue management
- Implemented retry logic and server-side session management with automatic disable on no_session error
- Added beforeunload handler with sendBeacon for final log submission
- Updated debug helper functions to support remote-only mode (logs sent without local console output)
- Removed obsolete README instruction about manually setting wardriving channel
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| content/wardrive.js | Added complete remote debug logging infrastructure including configuration constants, queue management, console overrides, batch submission with retry logic, and integration with existing debug helper functions |
| README.md | Removed obsolete instruction about manually configuring wardriving channel on companion device (now handled automatically by app) |
| debugLogQueue.rateResetTimerId = setInterval(() => { | ||
| debugLogQueue.logsThisSecond = 0; | ||
|
|
||
| // Log when grace period expires (once at 15 seconds) |
There was a problem hiding this comment.
The comment states "Log when grace period expires (once at 15 seconds)" but REMOTE_DEBUG_GRACE_PERIOD_MS is 10000ms (10 seconds). This inconsistency should be corrected to say "once at 10 seconds" to match the actual grace period duration.
| debugkey: REMOTE_DEBUG_KEY, | ||
| data: debugLogQueue.messages | ||
| }); | ||
| navigator.sendBeacon(REMOTE_DEBUG_ENDPOINT, payload); |
There was a problem hiding this comment.
When using navigator.sendBeacon with a string payload, the Content-Type header is set to "text/plain" by default, which may cause the server to reject the request if it expects "application/json". Consider creating a Blob with the correct Content-Type: new Blob([payload], { type: 'application/json' }) instead of passing the raw JSON string.
| navigator.sendBeacon(REMOTE_DEBUG_ENDPOINT, payload); | |
| const beaconBody = new Blob([payload], { type: 'application/json' }); | |
| navigator.sendBeacon(REMOTE_DEBUG_ENDPOINT, beaconBody); |
| function queueRemoteDebugLog(level, args) { | ||
| if (!REMOTE_DEBUG_ENABLED) return; | ||
|
|
||
| // Grace period check - bypass rate limiting for first 15 seconds |
There was a problem hiding this comment.
The inline comment says "bypass rate limiting for first 15 seconds" but REMOTE_DEBUG_GRACE_PERIOD_MS is 10000ms (10 seconds). This comment should be updated to accurately state "first 10 seconds" to match the constant value.
| await new Promise(resolve => setTimeout(resolve, 1000)); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
If both retry attempts fail (neither succeeds nor gets a no_session error), the messages in messagesToSend are permanently lost because they were already removed from debugLogQueue.messages at line 122. Consider re-queuing failed messages back to debugLogQueue.messages so they can be retried in the next flush cycle, similar to how the wardrive API queue handles failures.
| // If we reach here, both attempts failed without a no_session disabling remote debug. | |
| // Re-queue the messages so they can be retried on the next flush cycle. | |
| if (REMOTE_DEBUG_ENABLED && messagesToSend.length > 0) { | |
| // Prepend the failed batch ahead of any new messages queued during this submit | |
| debugLogQueue.messages = messagesToSend.concat(debugLogQueue.messages); | |
| originalConsoleWarn( | |
| `[REMOTE DEBUG] Re-queued ${messagesToSend.length} debug logs after failed submit attempts` | |
| ); | |
| } |
| // These overrides call the original method AND queue for remote submission | ||
| console.log = function(...args) { | ||
| originalConsoleLog(...args); | ||
| queueRemoteDebugLog('log', args); | ||
| }; | ||
|
|
||
| console.warn = function(...args) { | ||
| originalConsoleWarn(...args); | ||
| queueRemoteDebugLog('warn', args); | ||
| }; | ||
|
|
||
| console.error = function(...args) { | ||
| originalConsoleError(...args); | ||
| queueRemoteDebugLog('error', args); | ||
| }; |
There was a problem hiding this comment.
The console method overrides (console.log, console.warn, console.error) are always active regardless of whether REMOTE_DEBUG_ENABLED is true. This means every console call goes through an extra function call even when remote debugging is disabled. Consider conditionally applying these overrides only when REMOTE_DEBUG_ENABLED is true to avoid unnecessary overhead in production.
| // These overrides call the original method AND queue for remote submission | |
| console.log = function(...args) { | |
| originalConsoleLog(...args); | |
| queueRemoteDebugLog('log', args); | |
| }; | |
| console.warn = function(...args) { | |
| originalConsoleWarn(...args); | |
| queueRemoteDebugLog('warn', args); | |
| }; | |
| console.error = function(...args) { | |
| originalConsoleError(...args); | |
| queueRemoteDebugLog('error', args); | |
| }; | |
| // These overrides call the original method AND queue for remote submission, | |
| // but are only applied when remote debugging is enabled to avoid overhead. | |
| if (REMOTE_DEBUG_ENABLED) { | |
| console.log = function(...args) { | |
| originalConsoleLog(...args); | |
| queueRemoteDebugLog('log', args); | |
| }; | |
| console.warn = function(...args) { | |
| originalConsoleWarn(...args); | |
| queueRemoteDebugLog('warn', args); | |
| }; | |
| console.error = function(...args) { | |
| originalConsoleError(...args); | |
| queueRemoteDebugLog('error', args); | |
| }; | |
| } |
| const REMOTE_DEBUG_FLUSH_INTERVAL_MS = 15000; // Flush every 15 seconds | ||
| const REMOTE_DEBUG_RATE_LIMIT = 20; // Max logs per second | ||
| const REMOTE_DEBUG_RATE_RESET_MS = 1000; // Rate limit reset interval | ||
| const REMOTE_DEBUG_GRACE_PERIOD_MS = 10000; // Grace period before rate limiting starts (15 seconds) |
There was a problem hiding this comment.
The comment states "Grace period before rate limiting starts (15 seconds)" but the constant value is 10000ms (10 seconds). This inconsistency could confuse developers. The comment should be updated to match the actual value of 10 seconds.
|
|
||
| /** | ||
| * Queue a log message for remote debug submission | ||
| * Handles rate limiting (10/sec) and batch size limits |
There was a problem hiding this comment.
The JSDoc comment states "Handles rate limiting (10/sec)" but the constant REMOTE_DEBUG_RATE_LIMIT is set to 20. The documentation should be updated to accurately reflect the rate limit of 20 logs per second.
| * Handles rate limiting (10/sec) and batch size limits | |
| * Handles rate limiting (20/sec) and batch size limits |
Key Highlights:
🎯 Remote Debug Logging System
Server-side log collection for production debugging
Rate limiting (20 logs/sec after 10s grace period)
Batch submission (100 logs max, 15s intervals)
Session management with server-side disable capability
Works in remote-only or combined local+remote mode